Add multiple C-API version infrastructure and C-API v1.1.0a.1#71
Add multiple C-API version infrastructure and C-API v1.1.0a.1#71
Conversation
|
@jbreue16 Is this the pull request you worked on Friday where you added the new C-API versioning to the cpp site? It’s cadet/CADET-Core#366. |
| _fields_ = _setup_api(_signatures_) | ||
|
|
||
|
|
||
| class CADETAPI_V1_0_0(ctypes.Structure): |
There was a problem hiding this comment.
If I recall correctly, the idea is to call the _set_api function with a string instead of the dictionary of signatures. Then, within _set_api, compare which string is greater.
What’s the benefit of doing it this way?
There was a problem hiding this comment.
yes exactly, but not using a string but this other type that Jo mentioned which supports comparisons of PEP440 version numbers. The benefit is that we can use this to successively add functions introduced in later (minor or pre-release) versions instead of repeating the whole list
There was a problem hiding this comment.
from packaging.version import Version
latest = "5.1.0"
current = Version("5.2.0a_1")
if current > Version("5.1.0"):
print("Something that was implemented in 5.2.0")
| class CADETAPI_V1_0_0(ctypes.Structure): | ||
| """Mimic cdtAPIv1.0.0 struct of CADET C-API in ctypes.""" | ||
| _fields_ = _setup_api("1.0.0") | ||
| # class CADETAPI_V1_0_0(ctypes.Structure): |
There was a problem hiding this comment.
I dont see a difference here?
| self._cadet_capi_version = cdtGetLatestCAPIVersion().decode('utf-8') | ||
|
|
||
| # Check which C-API is provided by CADET (given the current install path) | ||
| #current suppurted versions are: 1.0.0 and 1.1.0_a1 |
| if version == Version("1.0.0"): | ||
| signatures = dict(CADET_API_V1_SIGNATURES.signatures_1_0_0) | ||
|
|
||
| elif version >= Version("1.1.0"): |
There was a problem hiding this comment.
v1.1.0a1 is smaller than v1.1.0
| fields = [] | ||
|
|
||
| signatures = {} | ||
| if version == Version("1.0.0"): |
There was a problem hiding this comment.
I think we can put a > 1.0.0 && < 2.0.0 here and then not set signatures = dict(CADET_API_V1_SIGNATURES.signatures_1_0_0) for the other cases
Also, we could add a last case with < 1.0.0 || > 1.1.0a1 that raises an exception? @schmoelder
|
This would be my suggestions to implement the versioning and timeout in Key points:
|
AntoniaBerger
force-pushed
the
feature/CAPIversioning
branch
from
0bd1ad7 to
59130ee
Compare
|
@AntoniaBerger whats missing here?
With "latest c-api version" you mean the latest one implemented on the cadet-core side, which we read from |
Looks good to me. If you want to do this in a separate PR, then
Yes
Yes, but in our case, it is 1.0.0, is it not? But yes, it should maybe not be hard-coded.
I would support two full versions and give a warning that we will not support this lower version in the future. |
jbreue16
left a comment
jbreue16
left a comment
There was a problem hiding this comment.
only a unit test is missing now, locally ive already tested with a cadet simulation for different capi versions
3 participants
This PR adds infrastructure to implement support for multiple C-API versions and v1.1.0a1
The str lookup for the version number should be changed to the type @schmoelder suggested to make comparisons (
>, < =) workto be continued by @AntoniaBerger